Add fetcherKey APIs and update fetcher persistence architecture #10949
Conversation
🦋 Changeset detectedLatest commit: 65d6b8c The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
brophdawg11
changed the title
packages/react-router-dom/index.tsx
Outdated
| type FetchersContextObject = { | ||
| data: Map<string, any>; | ||
| register: (key: string) => void; | ||
| unregister: (key: string) => void; | ||
| }; |
There was a problem hiding this comment.
This context holds the fetcher data handed off from the router and exposes it for useFetcher
| newState.fetchers.forEach((fetcher, key) => { | ||
| if (fetcher.data !== undefined) { | ||
| fetcherData.current.set(key, fetcher.data); | ||
| } | ||
| }); |
There was a problem hiding this comment.
One-time fetcher data handoff. As soon as a router fetcher returns to idle, it comes through here one time and then is deleted inside the router. We stick the data into our fetcherData Map and clean it up when fetcherRefs returns to zero for a given key.
Need to look into cleanup behavior for fetchers that return data but never have a useFetcher register to grab it...
| fromRouteId: currentRouteId, | ||
| unstable_viewTransition: options.unstable_viewTransition, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Aaand we're back to one useSubmit 🤣 .
| @@ -1504,23 +1563,10 @@ export function useFormAction( | |||
| return createPath(path); | |||
| } | |||
|
|
|||
| function createFetcherForm(fetcherKey: string, routeId: string) { | |||
There was a problem hiding this comment.
Inlined in useFetcher
| let [fetcherKey, setFetcherKey] = React.useState<string>(key || ""); | ||
| if (!fetcherKey) { | ||
| setFetcherKey(getUniqueFetcherId()); | ||
| } |
There was a problem hiding this comment.
This should be the right way to assign the fetcher keys, we had some weird behaviors with the old approach of incrementing in a state initializer.
| // Register/deregister with FetchersContext | ||
| React.useEffect(() => { | ||
| register(fetcherKey); | ||
| return () => unregister(fetcherKey); | ||
| }, [fetcherKey, register, unregister]); |
There was a problem hiding this comment.
Register and unregister so we can ref count fetchers for cleanup
|
This has been superseded by: |
Implements the updated version of remix-run/remix#7698
Supersedes #10945
Todo:
router.stateentirely? Right now they remain in there when they return toidlefor onesubscribecall so we can hand-off the data to the React layer. However, would it make sense to remove them before calling subscribers and pass the data in a subsequent argument?databut never have auseFetcherregister to grab it so they never get a ref countkeyonuseFetchers